backport: merge bitcoin#31100, #31626, #31840, #30584, #31982, #32086, #31998, #32505, #32568, #32690, #32731, #32716, #32837, #32266, #30095, #30137, #33580, partial bitcoin#30454 (build backports: part 4) #6919
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
kwvg
changed the title
Checksums for d7075f9 |
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6919.d7075f9a. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6919.d7075f9a. The image should be on dockerhub soon. |
|
This pull request has conflicts, please rebase. |
, bitcoin#30387, bitcoin#31461, bitcoin#31484, bitcoin#31498, bitcoin#31552, bitcoin#31627, bitcoin#30774, bitcoin#31125, bitcoin#31661, bitcoin#31800, bitcoin#31500, bitcoin#33494, partial bitcoin#30940 (build backports: part 3) 8e07d33 merge bitcoin#33494: Update URL for `qrencode` package source tarball (Kittywhiskers Van Gogh) 3dd5c1c merge bitcoin#31500: Fix compiling `libevent` package on NetBSD (Kittywhiskers Van Gogh) be4e492 merge bitcoin#31800: Avoid using the `-ffile-prefix-map` compiler option (Kittywhiskers Van Gogh) 721da49 merge bitcoin#31661: Override default build type for `libevent` (Kittywhiskers Van Gogh) 802232d merge bitcoin#31125: add *FLAGS to gen_id (Kittywhiskers Van Gogh) 29c3c06 merge bitcoin#30774: Qt 5.15.16 (Kittywhiskers Van Gogh) 3dbc289 merge bitcoin#31627: Fix spacing issue (Kittywhiskers Van Gogh) 78ccada merge bitcoin#31552: Update capnproto to 1.1.0 (Kittywhiskers Van Gogh) aac17f3 merge bitcoin#31498: Ignore prefix directory on OpenBSD (Kittywhiskers Van Gogh) aa28a18 merge bitcoin#31484: update capnproto to 1.0.2 (Kittywhiskers Van Gogh) de2dc16 merge bitcoin#31461: add `-g` to *BSD_debug flags (Kittywhiskers Van Gogh) 31881e6 partial bitcoin#30940: Fix build with `MULTIPROCESS=1` in Guix environment (Kittywhiskers Van Gogh) f73021a merge bitcoin#30387: use c++ compiler rather than c compiler for binary checks (Kittywhiskers Van Gogh) 44ec95a merge bitcoin#29895: remove bzip2 from deps (Kittywhiskers Van Gogh) 86004c5 merge bitcoin#29249: add NM output to gen_id (Kittywhiskers Van Gogh) dc24acb refactor: point Qt URL to `archives` subdirectory (Kittywhiskers Van Gogh) 13e1bfe refactor: move `windres` fix to `hosts/mingw32.mk` (Kittywhiskers Van Gogh) b01440d merge bitcoin#28900: remove mingw-w64 install for "older" systems (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for #6919 * The current windres fix was introduced by [dash#6294](#6294) to fix mingw32 builds, the problem with the fix is that it assumes the target triple is fixed (i.e. `x86_64-w64-mingw32`), this may not hold true in the long run as Windows for ARM support is currently being tracked upstream (see [bitcoin#31388](bitcoin#31388)). To mitigate this, the fix has been generalised by setting the `WINDRES` variable, which is checked by `configure.ac` ([source](https://github.com/dashpay/dash/blob/f7dad69eab573c179060ff9eb1bbaccb317de6d3/configure.ac#L835)). * This fix had the effect of breaking detection (see error below) as `test -f` cannot traverse through `PATH` ([source](https://pubs.opengroup.org/onlinepubs/9799919799/utilities/test.html)), this has been resolved by using `command -v`, which is a better fit ([source](https://pubs.opengroup.org/onlinepubs/9799919799/utilities/command.html)) ``` make[1]: Entering directory '/distsrc-base/distsrc-23.0.0-rc.3-125-gca749d4d0d58-x86_64-w64-mingw32/src' make[2]: Entering directory '/distsrc-base/distsrc-23.0.0-rc.3-125-gca749d4d0d58-x86_64-w64-mingw32/src' CXX dashd-bitcoind.o windres x86_64-w64-mingw32-windres not found, but is required to compile windows resource files ``` * Versions below Qt 6.5 are considered (as of this writing), archived ([source](https://download.qt.io/official_releases/qt/)), this results in fetch failures that result in more usage of the cache fallback when trying to fetch Qt 5.15, which is now located in the archives ([source](https://download.qt.io/archive/qt/5.15/)). The URL has been updated to reflect the same. * While upstream reverted [bitcoin#33494](bitcoin#33494) with [bitcoin#33577](bitcoin#33577), the reasoning was to do with their cache and its interaction with the release process. As the underlying rationale for the revert doesn't match our case, we can retain the backport. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 8e07d33 Tree-SHA512: 3356603fb1f74470656f3df233100da8ff722dd95926c25a251fe542afc85876d6517a45bb015863003ffe485d3f17241e1e5e73fbbcc9b5e90729b429272300
Checksums for 15fe09a |
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6919.15fe09a9. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6919.15fe09a9. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6919.44b14852. A new comment will be made when the image is pushed. |
Checksums for 44b1485 |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6919.44b14852. The image should be on dockerhub soon. |
WalkthroughBroadens FreeBSD handling across build scripts and depends, removes configure-time thread_local detection, changes thread-name storage from thread-local std::string to a per-thread fixed char buffer and updates related APIs and call sites, introduces multiple package/patch additions and adjustments (capnp, libevent, libmultiprocess, qt FreeBSD wiring), and updates config.{sub,guess}, M4, Makefiles, documentation, and related build helpers. Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Key areas requiring careful review:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
depends/README.md (1)
100-100: Optional: Consider hyphenating compound modifier.The phrase "-flto related options" could be written as "-flto-related options" for improved readability, though this is a minor style consideration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
build-aux/m4/bitcoin_qt.m4(1 hunks)configure.ac(1 hunks)depends/Makefile(1 hunks)depends/README.md(4 hunks)depends/builders/freebsd.mk(1 hunks)depends/builders/openbsd.mk(1 hunks)depends/config.guess(8 hunks)depends/config.sub(20 hunks)depends/funcs.mk(1 hunks)depends/hosts/darwin.mk(1 hunks)depends/packages/capnp.mk(2 hunks)depends/packages/libevent.mk(2 hunks)depends/packages/native_capnp.mk(1 hunks)depends/packages/native_libmultiprocess.mk(1 hunks)depends/packages/packages.mk(1 hunks)depends/packages/qt.mk(2 hunks)depends/packages/xproto.mk(1 hunks)depends/patches/capnp/abi_placement_new.patch(1 hunks)depends/patches/capnp/fix_openbsd_build.patch(1 hunks)depends/patches/libevent/winver_fixup.patch(1 hunks)depends/patches/native_capnp/fix_openbsd_build.patch(1 hunks)doc/build-windows.md(1 hunks)doc/design/multiprocess.md(4 hunks)src/init/common.cpp(0 hunks)src/sync.cpp(2 hunks)src/test/util/setup_common.h(1 hunks)src/test/util_threadnames_tests.cpp(0 hunks)src/util/threadnames.cpp(2 hunks)src/util/threadnames.h(1 hunks)
💤 Files with no reviewable changes (2)
- src/test/util_threadnames_tests.cpp
- src/init/common.cpp
🧰 Additional context used
📓 Path-based instructions (4)
depends/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the depends directory (dependency build system)
Files:
depends/builders/openbsd.mkdepends/packages/native_capnp.mkdepends/funcs.mkdepends/patches/capnp/fix_openbsd_build.patchdepends/patches/libevent/winver_fixup.patchdepends/builders/freebsd.mkdepends/packages/capnp.mkdepends/config.subdepends/packages/libevent.mkdepends/config.guessdepends/packages/packages.mkdepends/hosts/darwin.mkdepends/packages/xproto.mkdepends/patches/native_capnp/fix_openbsd_build.patchdepends/patches/capnp/abi_placement_new.patchdepends/packages/native_libmultiprocess.mkdepends/packages/qt.mkdepends/README.mddepends/Makefile
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/test/util/setup_common.hsrc/util/threadnames.hsrc/util/threadnames.cppsrc/sync.cpp
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/util/setup_common.h
doc/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/build-windows.mddoc/design/multiprocess.md
🧠 Learnings (6)
📓 Common learnings
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.
📚 Learning: 2025-09-09T13:04:06.013Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/compat/compat.h:0-0
Timestamp: 2025-09-09T13:04:06.013Z
Learning: WSAEISCONN is only used within WIN32 conditional compilation blocks in the codebase, so it doesn't require errno mapping for non-Windows platforms in src/compat/compat.h.
Applied to files:
depends/patches/libevent/winver_fixup.patch
📚 Learning: 2025-01-06T09:51:03.167Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: depends/patches/gmp/include_ldflags_in_configure.patch:557-621
Timestamp: 2025-01-06T09:51:03.167Z
Learning: The `GMP_GCC_ARM_UMODSI` macro checks only the compiler version, and `GMP_GCC_MIPS_O32` relies on the `-mabi=32` flag. Therefore, `$LDFLAGS` is irrelevant to these tests.
Applied to files:
configure.acdepends/packages/qt.mk
📚 Learning: 2025-10-25T07:08:51.918Z
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.
Applied to files:
depends/packages/native_libmultiprocess.mk
📚 Learning: 2025-07-20T18:42:49.794Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to depends/** : Unless specifically prompted, avoid making changes to the depends directory (dependency build system)
Applied to files:
depends/README.mddepends/Makefile
📚 Learning: 2025-07-14T10:11:05.011Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6754
File: contrib/containers/guix/docker-compose.yml:18-19
Timestamp: 2025-07-14T10:11:05.011Z
Learning: In the Guix build process for Dash Core, the `guix.sigs` directory requires write access as signatures are written to it during the build process, and `dash-detached-sigs` may be updated with `git pull` operations, so both directories need rw permissions in the Docker volume mounts.
Applied to files:
doc/build-windows.md
🧬 Code graph analysis (1)
src/util/threadnames.h (1)
src/util/threadnames.cpp (6)
ThreadRename(59-63)ThreadRename(59-59)ThreadSetInternalName(65-68)ThreadSetInternalName(65-65)ThreadGetInternalName(49-49)ThreadGetInternalName(49-49)
🪛 checkmake (0.2.2)
depends/Makefile
[warning] 74-74: Target "host" should be declared PHONY.
(phonydeclared)
[warning] 76-76: Target "host" should be declared PHONY.
(phonydeclared)
🪛 LanguageTool
depends/README.md
[grammar] ~100-~100: Use a hyphen to join words.
Context: ...ons needed for LTO. Does not add -flto related options to *FLAGS. - `NO_HARDEN=...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (31)
depends/packages/native_libmultiprocess.mk (1)
3-3: Dependency source updated to official Bitcoin Core repository.The download source has been migrated from the chaincodelabs fork to the official Bitcoin Core libmultiprocess repository. This is consistent with the documentation updates in the PR.
doc/design/multiprocess.md (2)
22-22: Link references updated to official Bitcoin Core repository.Documentation links have been consistently updated from chaincodelabs to bitcoin-core libmultiprocess throughout the file, aligning with the dependency package change.
Also applies to: 35-35
39-56: Expanded documentation provides clearer architecture overview.The documentation has been enhanced to better explain the cross-process interface implementation, including clarification that proxy classes are thin wrappers around Cap'n Proto client/server classes handling serialization and socket communication. This improves clarity for developers understanding the multiprocess architecture.
src/util/threadnames.h (1)
15-22: LGTM: API signatures updated consistently.The function signatures have been updated to support the new fixed-buffer storage strategy:
ThreadRenameandThreadSetInternalNamenow acceptconst std::string&(more flexible than rvalue reference)ThreadGetInternalNamereturns by value (necessary since internal storage is now a char buffer)These changes align correctly with the implementation in src/util/threadnames.cpp.
src/sync.cpp (2)
44-49: LGTM: Thread name now stored by value.The CLockLocation constructor and member have been updated to store the thread name by value instead of by reference. This change is necessary because
util::ThreadGetInternalName()now returnsstd::stringby value (from the fixed-size char buffer). The use ofstd::movecorrectly transfers ownership of the temporary string.
67-67: LGTM: Member declaration matches the move-based initialization.The member type correctly changed from
const std::string&toconst std::stringto store the value directly, matching the move-based initialization in the constructor.src/util/threadnames.cpp (4)
42-48: LGTM: Clear documentation of design rationale.The comment explains the use of a char array instead of std::string to avoid destructor complications when threads exit, with a reference to the specific FreeBSD bug. This is valuable documentation for future maintainers.
48-49: LGTM: Safe thread-local storage with proper return.The fixed-size buffer (128 bytes) is thread-local, preventing data races.
ThreadGetInternalNamecorrectly constructs and returns a std::string from the null-terminated buffer.
52-57: LGTM: Safe bounded string copy with null termination.The implementation correctly:
- Limits copy to 127 characters (leaving room for null terminator)
- Uses
std::memcpywith calculated bounds- Explicitly null-terminates the buffer
Thread names longer than 127 characters will be silently truncated, which is acceptable given that OS-level thread names are typically limited to 15-16 characters anyway.
59-68: LGTM: Function signatures updated consistently.Both
ThreadRenameandThreadSetInternalNamenow acceptconst std::string&instead ofstd::string&&, providing more flexibility while maintaining efficiency through the internalSetInternalNamefunction.src/test/util/setup_common.h (1)
51-52: LGTM: Documentation updated to reflect new threading approach.The comment has been simplified to remove the suggestion of using
thread_local, aligning with the broader PR's shift away from thread_local variables toward runtime-configurable per-thread handling.depends/funcs.mk (1)
36-39: LGTM: Fallback now uses consistent filename argument.The change makes the fallback invocation consistent with the primary call on line 37, where
$(4)(the actual filename) is passed for both the third and fourth arguments tofetch_file_inner. This ensures the fallback download path uses the same file naming convention.depends/hosts/darwin.mk (1)
22-22: LGTM: Adds llvm-objcopy detection for Darwin.The addition follows the established pattern for tool detection in this file and is consistent with the broader Darwin/BSD tooling improvements in this PR.
depends/packages/xproto.mk (1)
24-28: LGTM: Fixes mkdir detection on Alpine Linux.The explicit
MKDIRPROG="mkdir -p"setting works around platform-specific detection issues and prevents parallelism problems during installation. The comment clearly documents the rationale.depends/builders/freebsd.mk (1)
1-9: LGTM: Adds FreeBSD host support.The changes appropriately configure FreeBSD as both a build and host platform:
- Updates
SHA256SUMto usesha256sum(standard FreeBSD command)- Sets
clang/clang++as default compilers for FreeBSD hosts- Consistent with the existing FreeBSD builder configuration
These align with the PR's objective to extend FreeBSD support in the build system.
depends/patches/native_capnp/fix_openbsd_build.patch (1)
1-71: LGTM: Patches capnp for OpenBSD compatibility.This patch correctly addresses two OpenBSD build issues:
Lines 1-34: Broadens
netinet/in.hinclusion to all non-Windows systems, not just FreeBSD. This is more portable and fixes OpenBSD builds.Lines 36-71: Excludes OpenBSD from
KJ_USE_KQUEUEbecause OpenBSD's kqueue doesn't support user event filters (EVFILT_USER), causing build failures. Falls back to poll()-based implementation.Both changes reference upstream capnproto discussions and follow standard patch format.
configure.ac (1)
965-968: LGTM: Adds FreeBSD host detection.The new FreeBSD case appropriately sets
TARGET_OS=freebsdwhen building for FreeBSD hosts. This follows the established pattern for other operating systems and integrates with the FreeBSD support added throughout this PR.depends/Makefile (1)
74-80: LGTM: Improves host override handling.The reordered initialization logic now properly supports HOST overrides:
hostis initially derived frombuildHOSTcan override this value (or defaults toBUILD)canonical_hostis computed from the (potentially overridden)HOSTThis provides more flexible cross-compilation support and aligns with the FreeBSD host detection added in this PR.
Note: The static analysis warnings about PHONY declarations are false positives—these are variable assignments, not make targets.
doc/build-windows.md (2)
30-31: LGTM: Consolidates dependency documentation.Referencing the centralized dependency documentation in
depends/README.mdanddependencies.mdreduces duplication and ensures consistency across the documentation.
37-51: LGTM: Documents WSL Win32 interoperability workaround.The added WSL note correctly identifies a real build issue where Win32 application launch support can cause unexpected behavior (error dialogs, library issues) during configure. The documented workaround (temporarily disabling binfmt_misc Win32 support) is appropriate and includes both disable and re-enable steps.
depends/packages/capnp.mk (1)
7-20: LGTM: Patch workflow properly implemented.The patch declarations and preprocessing commands correctly mirror the pattern used in native_capnp.mk, with consistent use of
patch -p2for both patches.depends/packages/packages.mk (1)
13-13: LGTM: FreeBSD Qt packages properly aliased.The FreeBSD Qt package set correctly reuses the Linux Qt packages, maintaining parity across platforms consistent with the approach in qt.mk.
build-aux/m4/bitcoin_qt.m4 (1)
151-153: LGTM: FreeBSD correctly added to XCB platform check.The conditional properly extends Linux behavior to FreeBSD using the
-ooperator, enabling QT_QPA_PLATFORM_XCB and XCB integration plugin for FreeBSD builds.depends/packages/native_capnp.mk (1)
7-17: LGTM: Patch workflow correctly implemented.The patch declaration and preprocessing command are consistent with the approach in capnp.mk, properly using
patch -p2to apply the OpenBSD build fix.depends/patches/capnp/fix_openbsd_build.patch (1)
1-71: LGTM: Well-documented OpenBSD build fixes.Both patches are properly formatted with clear rationale:
- Patch 1/2 broadens netinet/in.h inclusion to all non-Windows systems
- Patch 2/2 correctly excludes OpenBSD from kqueue usage due to missing user event filter support
depends/packages/qt.mk (2)
7-8: LGTM: FreeBSD dependencies correctly aliased.The FreeBSD dependencies properly reuse Linux dependencies using simple assignment, maintaining consistency with the cross-platform approach.
163-163: LGTM: FreeBSD config options correctly reference Linux options.The recursive reference using
$$properly defers expansion of the Linux config options for FreeBSD builds.depends/patches/libevent/winver_fixup.patch (1)
1-122: LGTM: Windows header ordering correctly fixed.The patch properly ensures
_WIN32_WINNTandWIN32_LEAN_AND_MEANare defined before any Windows header inclusion across multiple libevent source files, preventing header inclusion order issues.depends/README.md (3)
1-6: LGTM: Improved documentation structure.The new "Depends build" header and introduction clearly describe the depends system and its cross-compilation support.
28-54: LGTM: BSD platform instructions added.The FreeBSD, NetBSD, and OpenBSD sections provide clear, platform-specific build instructions with appropriate package installation commands.
69-104: LGTM: Comprehensive dependency options documented.The new "Dependency Options" section clearly lists all available make variables and NO_* flags, improving usability for developers configuring custom builds.
…n make for macOS
…atch, correct SHA256SUM command)
Should prevent conflict in next commit
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6919.8e79c7ac. A new comment will be made when the image is pushed. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/util/threadnames.cpp (1)
9-13: Missing<algorithm>include forstd::min.This will fail to compile on some toolchains.
Apply:
#if defined(HAVE_CONFIG_H) #include <config/bitcoin-config.h> #endif -#include <cstring> +#include <cstring> +#include <algorithm> #include <string> #include <thread> #include <utility>
🧹 Nitpick comments (1)
build-aux/m4/bitcoin_qt.m4 (1)
151-154: Align static Qt XCB deps for FreeBSD too.You added FreeBSD to the XCB plugin check, but _BITCOIN_QT_CHECK_STATIC_LIBS still gates XcbQpa/XkbCommonSupport/etc. on TARGET_OS=linux only. This may miss static deps on FreeBSD and cause link errors.
Consider broadening that condition similarly:
- if test "$TARGET_OS" = "linux"; then + if test "$TARGET_OS" = "linux" -o "$TARGET_OS" = "freebsd"; then PKG_CHECK_MODULES([QT_INPUT], [${qt_lib_prefix}InputSupport], [QT_LIBS="$QT_INPUT_LIBS $QT_LIBS"]) PKG_CHECK_MODULES([QT_SERVICE], [${qt_lib_prefix}ServiceSupport], [QT_LIBS="$QT_SERVICE_LIBS $QT_LIBS"]) PKG_CHECK_MODULES([QT_XCBQPA], [${qt_lib_prefix}XcbQpa], [QT_LIBS="$QT_XCBQPA_LIBS $QT_LIBS"]) PKG_CHECK_MODULES([QT_XKBCOMMON], [${qt_lib_prefix}XkbCommonSupport], [QT_LIBS="$QT_XKBCOMMON_LIBS $QT_LIBS"]) elif test "$TARGET_OS" = "darwin"; thenBased on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
build-aux/m4/bitcoin_qt.m4(1 hunks)configure.ac(2 hunks)contrib/devtools/symbol-check.py(2 hunks)depends/Makefile(1 hunks)depends/README.md(4 hunks)depends/builders/freebsd.mk(1 hunks)depends/builders/openbsd.mk(1 hunks)depends/config.guess(8 hunks)depends/config.sub(20 hunks)depends/funcs.mk(1 hunks)depends/hosts/darwin.mk(1 hunks)depends/packages/capnp.mk(2 hunks)depends/packages/libevent.mk(2 hunks)depends/packages/native_capnp.mk(1 hunks)depends/packages/native_libmultiprocess.mk(1 hunks)depends/packages/packages.mk(1 hunks)depends/packages/qt.mk(2 hunks)depends/packages/xproto.mk(1 hunks)depends/patches/capnp/abi_placement_new.patch(1 hunks)depends/patches/capnp/fix_openbsd_build.patch(1 hunks)depends/patches/libevent/winver_fixup.patch(1 hunks)depends/patches/native_capnp/fix_openbsd_build.patch(1 hunks)doc/build-windows.md(1 hunks)doc/design/multiprocess.md(4 hunks)src/init/common.cpp(0 hunks)src/sync.cpp(2 hunks)src/test/util/setup_common.h(1 hunks)src/test/util_threadnames_tests.cpp(0 hunks)src/util/threadnames.cpp(2 hunks)src/util/threadnames.h(1 hunks)
💤 Files with no reviewable changes (2)
- src/test/util_threadnames_tests.cpp
- src/init/common.cpp
🚧 Files skipped from review as they are similar to previous changes (13)
- depends/patches/capnp/fix_openbsd_build.patch
- depends/builders/openbsd.mk
- src/test/util/setup_common.h
- depends/packages/native_capnp.mk
- depends/packages/xproto.mk
- src/util/threadnames.h
- depends/hosts/darwin.mk
- src/sync.cpp
- depends/packages/capnp.mk
- doc/design/multiprocess.md
- depends/packages/packages.mk
- depends/packages/libevent.mk
- doc/build-windows.md
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.
📚 Learning: 2025-10-25T07:08:51.918Z
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.
Applied to files:
depends/packages/native_libmultiprocess.mk
📚 Learning: 2025-01-06T09:51:03.167Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: depends/patches/gmp/include_ldflags_in_configure.patch:557-621
Timestamp: 2025-01-06T09:51:03.167Z
Learning: The `GMP_GCC_ARM_UMODSI` macro checks only the compiler version, and `GMP_GCC_MIPS_O32` relies on the `-mabi=32` flag. Therefore, `$LDFLAGS` is irrelevant to these tests.
Applied to files:
configure.acdepends/packages/qt.mkdepends/config.sub
📚 Learning: 2025-09-09T13:04:06.013Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/compat/compat.h:0-0
Timestamp: 2025-09-09T13:04:06.013Z
Learning: WSAEISCONN is only used within WIN32 conditional compilation blocks in the codebase, so it doesn't require errno mapping for non-Windows platforms in src/compat/compat.h.
Applied to files:
depends/patches/libevent/winver_fixup.patch
🪛 checkmake (0.2.2)
depends/Makefile
[warning] 74-74: Target "host" should be declared PHONY.
(phonydeclared)
[warning] 76-76: Target "host" should be declared PHONY.
(phonydeclared)
🪛 Flake8 (7.3.0)
contrib/devtools/symbol-check.py
[error] 166-166: continuation line missing indentation or outdented
(E122)
[error] 167-167: continuation line missing indentation or outdented
(E122)
🪛 LanguageTool
depends/README.md
[grammar] ~100-~100: Use a hyphen to join words.
Context: ...ons needed for LTO. Does not add -flto related options to *FLAGS. - `NO_HARDEN=...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
depends/README.md
21-21: Link text should be descriptive
(MD059, descriptive-link-text)
53-53: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: arm-linux-gnueabihf / Build depends
- GitHub Check: Lint / Run linters
- GitHub Check: x86_64-w64-mingw32 / Build depends
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
🔇 Additional comments (17)
depends/packages/native_libmultiprocess.mk (1)
3-3: LGTM: Source migrated to official Bitcoin Core organization.The download source has been updated from chaincodelabs to the official bitcoin-core organization, which improves trust and aligns with the repository migration. The successful automated builds confirm that the sha256 hash remains valid for the new source.
contrib/devtools/symbol-check.py (2)
166-167: LGTM! Flake8 warnings are false positives.The addition of
dbghelp.dlland the inline comment forbcrypt.dllare appropriate. Thedbghelp.dlllibrary provides debugging and stack trace functionality, which aligns with the exported symbols rationale mentioned in Line 289. The Flake8 E122 indentation warnings are false positives—the indentation is consistent with all other entries in the set.
289-290: Disabling the exported symbols check is well-documented.The Dash-specific comment clearly explains the rationale for commenting out the
EXPORTED_SYMBOLScheck. This change is consistent with the addition ofdbghelp.dll(Line 167) to support improved backtrace generation. Keeping the function definition forcheck_exported_symbols(lines 210-221) is reasonable in case the check needs to be re-enabled in the future.depends/README.md (2)
1-68: Strong documentation structure with clear OS-specific guidance and improved section organization.The restructuring enhances clarity by leading with "Depends build" and a descriptive introduction, then organizing content into logical sections (Usage per OS, configuration guidance, options). The new "Configuring Dash Core" section (lines 56–67) addresses a critical configuration step with concrete CONFIG_SITE examples. OS-specific sections maintain consistent formatting and are accurate.
100-100: No action required—text matches upstream Bitcoin Core.The line reads "Does not add -flto related options to *FLAGS." in both the Dash PR and Bitcoin Core's upstream master branch. Per backport policy, the text should be preserved as-is to maintain fidelity with the original upstream implementation and avoid merge conflicts in future backports, even though the compound adjective lacks hyphenation.
depends/patches/native_capnp/fix_openbsd_build.patch (2)
1-34: LGTM: Patch correctly fixes netinet/in.h inclusion for BSDs.The patch removes the FreeBSD-only guard and makes
netinet/in.havailable on all non-Windows systems, which should fix OpenBSD builds without affecting other platforms.
36-71: LGTM: Patch correctly excludes OpenBSD from kqueue path.The patch appropriately removes OpenBSD from the
KJ_USE_KQUEUEcondition since OpenBSD's kqueue implementation lacks user event filter support. The fallback to poll()-based implementation is the correct solution.depends/patches/capnp/abi_placement_new.patch (1)
1-70: LGTM: Patch correctly addresses GCC/Clang ABI incompatibility.This patch solves a real issue where GCC and Clang use different calling conventions for empty struct parameters. By moving the
PlacementNewparameter to the end ofoperator newandoperator delete, the code becomes ABI-compatible across both compilers, preventing potential segfaults when mixing compiler outputs without optimizations.The changes are minimal, focused, and technically correct:
- Reorders parameters in operator new/delete declarations
- Updates the call site in
ctorto match- Includes comprehensive explanation of the problem and solution
depends/patches/libevent/winver_fixup.patch (1)
1-122: Windows macro/include ordering changes look correct and safeThe patch consistently moves
_WIN32_WINNT(andWIN32_LEAN_AND_MEANwhere used) ahead of any Windows headers inevent_iocp.c,evthread_win32.c,evutil.c, andlistener.c, while preserving#ifdef _WIN32guards and keeping<winsock2.h>before<windows.h>.#if/#endifpairs are balanced, non-Windows builds are unaffected, and the behavioral changes (mainly makingWIN32_LEAN_AND_MEANand_WIN32_WINNTset once per TU instead of define/undef around headers) align with the stated intent and upstream backporting practices. I don’t see any issues to block this patch.
Based on learningsdepends/packages/qt.mk (2)
7-9: FreeBSD deps aliasing looks good.Mirroring Linux deps to FreeBSD is reasonable here. Please confirm all listed XCB/* deps build in your FreeBSD depends environment.
165-166: Indirection for FreeBSD config opts is fine; verify mkspec/platform flags.Using
:= $$($(package)_config_opts_linux)defers to the Linux opts at use time, so later toolchain-specific additions are inherited. Verify that-platform/-xplatform linux-*mkspecs work on FreeBSD in this Qt static build context.configure.ac (2)
949-951: LGTM: explicit FreeBSD target mapping.Clearer than lumping under generic BSD; no issues spotted.
1198-1211: LGTM: BSD execinfo detection via host pattern.Switching to
$hostmatches FreeBSD use-cases and avoids spurious Linux link flags.src/util/threadnames.cpp (1)
42-57: Name copy/truncation logic is OK.Bounded copy with explicit NUL is correct for 127-byte limit; no further issues.
depends/config.sub (1)
3-8: Upstream sync to 2024-05-27 looks good.No project-specific concerns.
depends/config.guess (1)
3-8: Upstream sync to 2024-07-27 looks good.Additions (LLVM libc, new OS cases) are benign.
depends/Makefile (1)
74-80: HOST defaulting and canonicalization logic reordered as part of build infrastructure backport.The changes move HOST defaulting to after the initial
hostassignment and add conditional override logic beforecanonical_hostderivation. This aligns with the broader build infrastructure refactoring (particularly FreeBSD support enhancements) across this backport series. The reordering allows HOST to be conditionally overridden before canonical host computation, which is intentional for the updated build initialization flow.Note: The static analysis warning that "target 'host' should be declared PHONY" is a false positive—these are variable assignments, not make targets, so the PHONY declaration warning does not apply.
As per learnings: For backport PRs from Bitcoin Core, backported changes should match upstream implementations exactly. This reordering appears consistent with the intended build infrastructure changes.
| build_freebsd_SHA256SUM = sha256sum | ||
| build_freebsd_DOWNLOAD = curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Ensure sha256sum is available on FreeBSD builders (or add fallback).
FreeBSD base ships sha256, not sha256sum. If Guix/Coreutils isn’t present on the FreeBSD builder, -c verification will fail.
Option A: verify sha256sum exists in your FreeBSD build env.
Option B: switch back to a portable tool or add a guarded fallback.
Based on learnings.
🏁 Script executed:
# Quick check on builder
command -v sha256sum || command -v shasum || command -v sha256Length of output: 0
I need to examine the codebase context to properly assess this issue. Let me gather more information about the build environment setup and how SHA256 tools are handled across different builders.
🏁 Script executed:
#!/bin/bash
# Show the freebsd.mk file content around lines 4-6
fd -t f 'freebsd.mk' -x cat {}
echo "---"
echo "Checking other builder files for SHA256SUM patterns:"
fd -t f '\.mk$' -path '*/builders/*' -x grep -l 'SHA256SUM' {}Length of output: 1345
🏁 Script executed:
#!/bin/bash
# Compare SHA256SUM definitions across builders
echo "=== SHA256SUM definitions in all builders ==="
fd -t f '\.mk$' -path '*/builders/*' -x grep -n 'SHA256SUM' {}
echo ""
echo "=== Checking for sha256/shasum/sha256sum references in build system ==="
rg -n 'sha256sum|shasum' --type-list | head -20
fd -t f \( -name '*.mk' -o -name '*.sh' -o -name 'Makefile' \) -x rg -l 'sha256sum|shasum' {}Length of output: 908
Let me fix the syntax errors and properly search the codebase:
🏁 Script executed:
#!/bin/bash
# Find all builder files
echo "=== All builder .mk files ==="
find depends/builders -name '*.mk' -type f
echo ""
echo "=== SHA256SUM definitions across all builders ==="
grep -n 'SHA256SUM' depends/builders/*.mkLength of output: 733
🏁 Script executed:
#!/bin/bash
# Search for actual usage of sha256sum in the codebase
echo "=== References to SHA256SUM variable ==="
rg -n 'SHA256SUM'
echo ""
echo "=== Direct sha256sum tool references ==="
rg -n 'sha256sum' --type sh --type mk
echo ""
echo "=== FreeBSD specific SHA256 references ==="
rg -n 'sha256' depends/builders/freebsd.mkLength of output: 8305
---
🌐 Web query:
```web
FreeBSD base system sha256sum vs sha256 command availability
💡 Result:
Short answer: FreeBSD historically provided a sha256 command (from the base's libmd/OpenSSL utilities) while the GNU-style sha256sum was not in base until more recently. Recent FreeBSD releases (e.g. FreeBSD 14 series) include a sha256sum manpage and the utility is available in base; older releases relied on sha256 / shasum (from ports/perl/coreutils) or openssl dgst. Check your installed release to see which is present (which sha256 sha256sum shasum). [1][2][3]
Sources:
- FreeBSD sha256/sha256sum manpage (FreeBSD 14.3 manpath). [1]
- Discussion/StackOverflow: FreeBSD provides md5/sha* utilities via base libraries (libmd / OpenSSL). [2]
- Project/commit discussion noting use of sha256sum in FreeBSD base (example: bitcoin depends change). [3]
🏁 Script executed:
#!/bin/bash
# Search for FreeBSD version specs or build environment docs
echo "=== Looking for FreeBSD version references ==="
rg -n 'freebsd.*14|freebsd.*13|freebsd.*12' -i
echo ""
echo "=== Looking for Guix or build environment setup ==="
fd -t f 'guix*' -o 'Dockerfile*' -o 'manifest*'
find . -name 'guix*' -o -name 'Dockerfile*' -o -name '*manifest*' 2>/dev/null | head -20
echo ""
echo "=== Check depends documentation ==="
ls -la depends/*.md depends/*.txt 2>/dev/null || echo "No docs found"
echo ""
echo "=== Check for environment/requirements files ==="
fd -t f 'requirements|README|INSTALL' depends/Length of output: 1524
🏁 Script executed:
#!/bin/bash
# Check git history for sha256sum change in freebsd.mk
echo "=== Recent changes to freebsd.mk ==="
git log --oneline -n 20 depends/builders/freebsd.mk 2>/dev/null || echo "Git history unavailable"
echo ""
echo "=== Full git blame for SHA256SUM line ==="
git blame depends/builders/freebsd.mk 2>/dev/null | grep -A1 -B1 'SHA256SUM' || echo "Git blame unavailable"Length of output: 660
🏁 Script executed:
#!/bin/bash
# Check what was the prior SHA256SUM setting before the recent change
echo "=== Prior state of freebsd.mk before sha256sum change ==="
git show 8d887c34abd~1:depends/builders/freebsd.mk 2>/dev/null | head -20 || echo "Cannot show prior state"
echo ""
echo "=== Commit message for the sha256sum change ==="
git log -1 --format=fuller 8d887c34abd 2>/dev/null || echo "Cannot show commit"
echo ""
echo "=== Check if there's any FreeBSD version pinning in build setup ==="
cat doc/build-freebsd.md 2>/dev/null || echo "No build-freebsd.md"Length of output: 4388
🏁 Script executed:
#!/bin/bash
# Check Guix manifest for FreeBSD or sha256sum/coreutils
echo "=== Guix manifest content ==="
cat contrib/guix/manifest.scm 2>/dev/null | head -100
echo ""
echo "=== Search for sha256sum in Guix files ==="
rg 'sha256sum|coreutils' contrib/guix/ contrib/containers/guix/Length of output: 5635
Revert sha256sum to portable shasum -a 256 or ensure coreutils availability.
This is a regression: the recent change (commit 8d887c3, Jan 9) switched FreeBSD from shasum -a 256 to sha256sum. However, FreeBSD 12.3 does not include sha256sum in base, and your build documentation explicitly targets FreeBSD 12.3. The prior shasum -a 256 was portable across BSD variants.
Either revert to shasum -a 256 (the documented target won't have sha256sum without coreutils), ensure coreutils is a declared FreeBSD build dependency, or update documentation to require FreeBSD 14+.
🤖 Prompt for AI Agents
In depends/builders/freebsd.mk around lines 4 to 6, the change replaced the
portable "shasum -a 256" with "sha256sum", which breaks builds on the documented
FreeBSD 12.3 target since sha256sum isn't in base; either revert
build_freebsd_SHA256SUM to use "shasum -a 256", or add a small portability fix:
detect availability of sha256sum and otherwise use "shasum -a 256" (or declare
coreutils as a required dependency and update the FreeBSD build docs to require
coreutils or a newer FreeBSD); implement one of these options and update the
README/docs to reflect the chosen requirement.
| $(call fetch_file_inner,$(1),$(FALLBACK_DOWNLOAD_PATH),$(4),$(4),$(5))) | ||
| endef |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm fallback filename mapping won’t break packages with renamed downloads.
Fallback now requests "$(FALLBACK_DOWNLOAD_PATH)/$(4)". If any package sets
Run to list packages that define custom download_file and compare:
🏁 Script executed:
#!/bin/bash
# List packages defining *_download_file and *_file_name and show differing values
rg -nP '^\s*\$\((\w+)\)_download_file\s*=' depends/packages | while read -r line; do
file=$(echo "$line" | cut -d: -f1)
pkg=$(sed -n 's/^package=\(.*\)$/\1/p' "$file")
df=$(sed -n 's/^\$\('"$pkg"'\)_download_file\s*=\s*\(.*\)$/\2/p' "$file")
fn=$(sed -n 's/^\$\('"$pkg"'\)_file_name\s*=\s*\(.*\)$/\2/p' "$file")
if [ -n "$df" ] && [ -n "$fn" ] && [ "$df" != "$fn" ]; then
echo "$pkg: download_file=$df file_name=$fn ($file)"
fi
doneLength of output: 0
🏁 Script executed:
#!/bin/bash
# First, check directory structure and list package files
echo "=== Package files in depends/packages ==="
find depends/packages -type f -name "*.mk" | head -10
echo ""
echo "=== Search for _download_file definitions ==="
rg '_download_file\s*=' depends/packages -A 1 -B 1 | head -30
echo ""
echo "=== Search for _file_name definitions ==="
rg '_file_name\s*=' depends/packages -A 1 -B 1 | head -30Length of output: 3870
Fallback filename mapping will break 4 packages with custom download filenames.
Verification found mismatched _download_file and _file_name in:
-
native_capnp.mk: download_file=
capnproto-c++-$(version).tar.gzvs file_name=capnproto-cxx-$(version).tar.gz - capnp.mk: inherits mismatch from native_capnp
-
qrencode.mk: download_file=
v$(version).tar.gzvs file_name=$(package)-$(version).tar.gz -
backtrace.mk: download_file=
$(version).tar.gzvs file_name=$(package)_$(version).tar.gz
The fallback now passes $(4) (file_name) where it should pass $(3) (download_file). This causes fallback mirror fetches to use local filenames instead of actual server filenames, resulting in 404 errors for these packages.
Correct the fallback call in depends/funcs.mk line 38 to pass $(3) instead of $(4) for the name parameter.
🤖 Prompt for AI Agents
In depends/funcs.mk around lines 38-39, the fallback call currently forwards
$(4) (local file_name) to fetch_file_inner which breaks packages whose download
filenames differ; change the argument to pass $(3) (download_file) instead of
$(4) so the fallback mirror fetch uses the actual server filename; update the
single call on that line to replace the name parameter from $(4) to $(3) and
keep all other arguments unchanged.
Checksums for 8e79c7a |
|
oh, indeed, I missed a couple of |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6919.8e79c7ac. The image should be on dockerhub soon. |
4 participants
Additional Information
Dependent on backport: merge bitcoin#28900, #29249, #29895, #30387, #31461, #31484, #31498, #31552, #31627, #30774, #31125, #31661, #31800, #31500, #33494, partial bitcoin#30940 (build backports: part 3) #6918
Dependency for backport: merge bitcoin#31048, #31099, #31172, #32400, #31608, #13151, #26415, #30321, #30263, #33489, partial bitcoin#29023, #30454, #32922, #27125 (build backports: part 5) #6927
dash#6966 broke Guix build post-compilation binary verification for ELF and PE binaries as
For PE binaries, with improved
libbacktracedetection, we are now building the capability in Windows binaries and that introduces a dependency ondbghelp.dll(source) that wasn't in thePE_ALLOWED_LIBRARIESallowlist, it has since been added.For ELF binaries, now that we use
-export-dynamicfor symbol preservation, theEXPORTED_SYMBOLScheck now fails. As the diagnostic value of retaining these symbols far exceeds the file size reduction, we have opted to comment out the test entirely.Breaking Changes
None expected.
Checklist